[Client encryption] Add synchronous initialization to CosmosDataEncryptionKeyProvider #5423
Conversation
MartinSarkany
requested review from
FabianMeiswinkel,
Pilchie,
kirankumarkolli and
sboshra
as code owners
MartinSarkany
changed the title
juraj-blazek
left a comment
juraj-blazek
left a comment
There was a problem hiding this comment.
We need to regenerate public API contracts, too
I'll update it after #5373 that fixes the contracts is merged. It is broken at the moment. |
But it should work for .net 6 at least. |
…ttps://github.com/MartinSarkany/azure-cosmos-dotnet-v3 into feature/cosmosdataencryptionkeyprovider-sync-init
| /// Initialize using an existing Cosmos DB container for storing wrapped DEKs. | ||
| /// </summary> | ||
| /// <param name="container">Existing Cosmos DB container.</param> | ||
| public void Initialize(Container container) |
There was a problem hiding this comment.
There are assumptions about the partition-key which were validated in initialization
Its even high impactful post initialization if CosmosDB is unavailable right?
There was a problem hiding this comment.
I added it to XML doc.
| } | ||
|
|
||
| /// <summary> | ||
| /// Initialize using an existing Cosmos DB container for storing wrapped DEKs. |
There was a problem hiding this comment.
Please include the container requirements (ex: partitionKeyDefinition)
And also call out right usage pattern
There was a problem hiding this comment.
I extended the XML doc with these details.
| { | ||
| this.ThrowIfAlreadyInitialized(); | ||
|
|
||
| this.container = container ?? throw new ArgumentNullException(nameof(container)); |
There was a problem hiding this comment.
Thoughts on handling non-existing containers on Fetch* API's? (i.e. on failure attempt re-create it)
There was a problem hiding this comment.
Concurrency control is also an important aspect
There was a problem hiding this comment.
I fixed the race condition.
However, I'm not sure about container re-creation as the container should exist after successful initialization and we probably can't re-create the same container if it's initialized using the Initialize() method. The caller has an option to react to failure by providing a new Container or using InitializeAsync() instead.
4 participants
Pull Request Template
Description
Adds synchronous initialization to CosmosDataEncryptionKeyProvider.
Depends on #5418 due to TestEncryptionKeyStoreProvider being introduced there.
Type of change
Please delete options that are not relevant.
Closing issues
closes #5400